Would we be able to remove the CumulativeEnergyExported and PeriodicEnergyExported embedded cluster defs now that we are just using the imported variation?

We could. I figured why not leave them in, but it's just as well to take them out

removed in 438b6252f08fba1d

This flag will have been persisted in the case on devices that have already joined to the platform and run this code. However, it wouldn't be needed anymore so we should set it to nil and remove it.

However, I don't think there are a lot/any devices like this yet, so in this case we are okay to just remove this, but just something to be aware of for the future.

To note: I have not removed anything- only renamed the word export to import.

Oh, nevermind I see what you mean. Thanks for pointing that out!

update on this: from the superset query counting all matter profiles in use, I'm seeing:
Matter Switch plug-power-energy-powerConsumption 510, so there are 510 of these devices that would have run this code already. Note: this code would only run if the device supports powerConsumption, so no need to count any devices that don't profile to this.

@ctowns Do you think we should add extra handling to transition these devices?

Could you see what device these are? If it's the eve energy plug, then those devices would use the eve-energy sub driver, so then we technically wouldn't need to worry about clean up for those since they wouldn't have run this code, they would have just used the sub driver.

@ctowns These wouldn't be for the eve-energy driver, since those devices still fingerprints to this power-energy-powerConsumption. You can see these in the query as well: Matter Switch power-energy-powerConsumption 9516. The 512 would only have come from the 1.3 support

thanks for clarifying! So is it reasonable to assume that the 510 devices just aren't working in terms of power reports right now? If we are reading the wrong attribute, then I would assume the data we are getting is wrong so I'm not sure what "transition" we'd need to do clean up wise since the data would've just been wrong. I assume we can't change it once it's made it to ST energy. So in that case, I guess the only clean up we could do would be setting the export fields to nil. FWIW, ultimately this would save just a few bytes of memory per device.

I am surprised there are this many devices using this driver already, I would suggest we figure out what these devices are and try to procure one so we can test this functionality with a real device! That could give us some more confidence moving forward.

Yes, that should be the case. In fact, if these devices are correctly using the 'Imported' attributes, we would not have read anything at all, so nothing would have even been reported to ST Energy in the first place, since the report reads TOTAL_IMPORTED_ENERGY, which is only set in the handlers.

Further, if anyone were incorrectly using the 'exported' attribute like us, there's this logic gate in the report:
if previous_imported_report and previous_imported_report.energy then energy_delta_wh = math.max(latest_total_imported_energy_wh - previous_imported_report.energy, 0.0) end
which will make sure it never reported a negative value.

Therefore, by this logic we shouldn't have reported anything incorrectly so far to ST Energy except sending a lot of nil values from the fact that we're never updating TOTAL_IMPORTED_ENERGY.

And as for saving a few bytes like you mentioned, I'm not sure this is worth it either.

Agreed, a couple bytes is unimportant especially for these low device counts. However, I think it is still a good idea to look into procuring one of the devices that actually uses the new clusters defs so we can do testing with this on a real device. This would also help with testing against older lua libs on a real device.

This st.matter.generated.zap_clusters prefix needs to be removed for this to work properly on hub running older lua libs where these definitions do not exist, same in the file below.

fixed